Cosmos: downgrade request-status error logs to debug - #4711
Cosmos: downgrade request-status error logs to debug#4711analogrelay with Copilot wants to merge 5 commits into
Conversation
Co-authored-by: analogrelay <7574+analogrelay@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Downgrades Cosmos HTTP-status logging to reduce noise from expected control-flow responses.
Changes:
- Changes pipeline abort logs from error to debug.
- Changes hedged terminal HTTP logs from warning to debug.
Show a summary per file
| File | Description |
|---|---|
operation_pipeline.rs |
Lowers terminal outcome log severity. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Medium
analogrelay
left a comment
There was a problem hiding this comment.
@copilot While you're fixing up the issues below, look out for any places where we're tracing "event name"-style strings like cosmos.hedge.terminal_http_error. Tracing events should have human-readable log-line-style messages.
There was a problem hiding this comment.
@copilot Fix this up to use a human-readable log line instead of an "event name" style. Something like non-retriable http error in hedging attempt
There was a problem hiding this comment.
Updated in 16ae25a. This now uses a human-readable message: non-retriable http error in hedging attempt.
There was a problem hiding this comment.
@copilot Fix this up to use a human-readable log line instead of an "event name" style. Something like non-retriable transport error in hedging attempt
There was a problem hiding this comment.
Updated in 16ae25a. This now uses a human-readable message: non-retriable transport error in hedging attempt.
Co-authored-by: analogrelay <7574+analogrelay@users.noreply.github.com>
Addressed in 16ae25a. Updated the touched hedging terminal tracing events to use human-readable log-line messages instead of event-name-style strings. |
There was a problem hiding this comment.
Do we need a changelog entry here?
There was a problem hiding this comment.
Additionally, I think this might introduce some conflict with the specs.
§10.4's level table and OTel-naming note are now partially contradicted.
| request_count = diagnostics.request_count(), | ||
| http_status = u16::from(status.status_code()), | ||
| sub_status = ?status.sub_status(), | ||
| "cosmos.hedge.terminal_http_error", |
There was a problem hiding this comment.
No migration note for anyone alerting on cosmos.hedge.terminal_http_error — string is gone. Breaking for log-based alerting, or never a supported surface? (§10.4 says "reserved," which cuts toward the latter.)
|
@copilot Can you address @NaluTripician 's two comments? Specifically:
Yes, but we don't need detail. It can simply say "Changes the messages and severity of some tracing events". It's early enough that I'm not worried about being hyper-accurate there. Add that CHANGELOG entry (ensure you reference this PR as all the others do)
Update relevant hedging specs for the change in event text. |
Co-authored-by: analogrelay <7574+analogrelay@users.noreply.github.com>
Addressed in 74c6479.
|
Create-if-not-existsand similar flows can intentionally treat non-2xx service responses (for example, 409 Conflict) as successful control flow. The Cosmos pipeline was emitting high-severity logs for these outcomes, creating noisyWARN/ERRORoutput under normal usage.Pipeline abort logging severity
operation_pipelineabort-path logging fromtracing::error!totracing::debug!for terminal operation outcomes that include typed Cosmos HTTP status.Hedged terminal HTTP outcome severity
cosmos.hedge.terminal_http_error) fromtracing::warn!totracing::debug!.Behavioral impact